-
Notifications
You must be signed in to change notification settings - Fork 1.3k
chore: Update font and line heights to latest design #8464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Build successful! 🎉 |
# Conflicts: # packages/@react-spectrum/s2/src/Menu.tsx
import ReactDOM from 'react-dom'; | ||
import {ReactNode, version as ReactVersion, useEffect, useMemo, useRef} from 'react'; | ||
import {useLocale} from 'react-aria'; | ||
import './fonts.css'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a use-case to not include these @font-face
rules? they are only loaded when actually used. If we want an opt-out, it'll be more complicated and won't be possible to include in Provider.
// Typekit ids for each CJK locale. These use dynamic subsetting, so cannot be loaded as CSS. | ||
// Because of the large size of the JS, we only download the script for the locale that is used. | ||
// Each of these kits includes regular, bold, extra bold, and black weights. Spectrum 2 does not use any lighter weights. | ||
let scripts = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could potentially allow these kit ids to be overridden via a prop in case teams want to optimize the fonts more (e.g. exclude certain weights they don't use). Add now or later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's super easy and we're confident about api for it and docs, then we can add it now. Otherwise I'm fine leaving it until later. They are only loaded on demand, so we're already in a better starting point with this PR
@@ -292,7 +292,6 @@ let keyboard = style<{size: 'S' | 'M' | 'L' | 'XL', isDisabled: boolean}>({ | |||
gridArea: 'keyboard', | |||
marginStart: 8, | |||
font: 'ui', | |||
fontWeight: 'light', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's normal weight in figma
@font-face { | ||
font-family: "adobe-clean-spectrum-vf"; | ||
src: url("https://use.typekit.net/af/b1226a/0000000000000000775c0485/31/l?primer=f592e0a4b9356877842506ce344308576437e4f677d7c9b78ca2162e6cad991a&fvd=n1&v=3") format("woff2"), url("https://use.typekit.net/af/b1226a/0000000000000000775c0485/31/d?primer=f592e0a4b9356877842506ce344308576437e4f677d7c9b78ca2162e6cad991a&fvd=n1&v=3") format("woff"), url("https://use.typekit.net/af/b1226a/0000000000000000775c0485/31/a?primer=f592e0a4b9356877842506ce344308576437e4f677d7c9b78ca2162e6cad991a&fvd=n1&v=3") format("opentype"); | ||
font-display: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we want swap. that will show the fallback font immediately and then swap to adobe clean once it loads. advantage: text is visible faster. disadvantage: there can be a layout shift. 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people will complain if there is a shift, they already complain in v3 about the fouc, this is less bad, but i think would get the same kind of complaint
Build successful! 🎉 |
Build successful! 🎉 |
according to spectrum
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once main is merged in an chromatic is re-run, i'll approve again
Setup for new adobe-clean-spectrum-vf font. Line heights are now calculated based on the font size, linearly interpolating between 1.3 and 1.15 as the font size grows. I did this with a calc rather than hard coding each line height for each font size and desktop/mobile scale. Font sizes are also now calculated based on the logarithmic scale, with an
--s2-font-size-base
variable controlling the base font size (14 on desktop, 17 on mobile).